Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Change preprocessing checks to be accurate for trigger types that depend on deployed state.

Type of Change

  • Bug fix

Testing

Tested manually.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 8, 2025 8:45pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 8, 2025

Greptile Overview

Greptile Summary

This PR fixes a critical ordering issue in preprocessing checks to prevent logs and costs from being created for undeployed workflows.

Key Changes:

  • Moved deployment status check to Step 2 (immediately after workflow validation) in preprocessExecution, before billing resolution and logging
  • Undeployed workflow rejections now return logCreated: false instead of true, avoiding unnecessary database writes and cost tracking
  • Added isTestMode option to checkWebhookPreprocessing() to allow test webhooks to skip deployment checks while still enforcing rate/usage limits
  • Test webhook endpoint now passes isTestMode: true to enable testing workflows in live/draft state before deployment
  • Removed fallback provider value in webhook execution (minor cleanup)

Impact:

  • Prevents cost increments and log entries for calls to undeployed workflows
  • Maintains proper enforcement of rate limits and usage limits for all workflows
  • Allows developers to test webhooks before deployment without requiring the workflow to be deployed

Confidence Score: 5/5

  • This PR is safe to merge with no risks identified
  • The change is a well-designed fix that correctly reorders preprocessing checks. The deployment check is logically placed before any state-modifying operations (logging, billing), which is the correct behavior. Test coverage is updated appropriately, and the isTestMode flag is properly threaded through the call chain with clear documentation.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/execution/preprocessing.ts 5/5 Deployment check now happens before logging/billing resolution, preventing logs and costs for undeployed workflows
apps/sim/lib/webhooks/processor.ts 5/5 Added isTestMode option to skip deployment check for test webhooks while preserving rate/usage limits
apps/sim/app/api/webhooks/test/[id]/route.ts 5/5 Test webhook endpoint now passes isTestMode flag to allow testing on live state without deployment requirement

Sequence Diagram

sequenceDiagram
    participant Client
    participant WebhookRoute
    participant Processor
    participant PreprocessExecution
    participant DB
    participant Logger
    participant RateLimiter
    participant BillingService

    Client->>WebhookRoute: POST webhook request
    WebhookRoute->>Processor: checkWebhookPreprocessing(isTestMode)
    Processor->>PreprocessExecution: preprocessExecution(checkDeployment=!isTestMode)
    
    Note over PreprocessExecution: STEP 1: Validate Workflow Exists
    PreprocessExecution->>DB: SELECT workflow
    DB-->>PreprocessExecution: workflow record
    
    Note over PreprocessExecution: STEP 2: Check Deployment (BEFORE logging!)
    alt Deployment check enabled AND workflow not deployed
        PreprocessExecution-->>Processor: Error (403, logCreated: false)
        Processor-->>WebhookRoute: NextResponse error
        WebhookRoute-->>Client: 403 Unauthorized
    else Deployment check passed or skipped
        Note over PreprocessExecution: STEP 3: Resolve Billing Actor
        PreprocessExecution->>BillingService: getWorkspaceBilledAccountUserId()
        BillingService-->>PreprocessExecution: actorUserId
        
        Note over PreprocessExecution: STEP 4-6: Rate & Usage Limits
        PreprocessExecution->>RateLimiter: checkRateLimitWithSubscription()
        RateLimiter-->>PreprocessExecution: rate limit result
        
        alt Rate limit exceeded
            PreprocessExecution->>Logger: Log preprocessing error
            PreprocessExecution-->>Processor: Error (429, logCreated: true)
        else All checks passed
            PreprocessExecution-->>Processor: Success
            Processor-->>WebhookRoute: null (no error)
            WebhookRoute->>WebhookRoute: queueWebhookExecution()
            WebhookRoute-->>Client: 200 OK
        end
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@icecrasher321 icecrasher321 merged commit b7a1e8f into staging Dec 9, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/execution-preproc-check branch December 10, 2025 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants